Docs: add polar, radar, radial bar, pie, and wind rose guides - #371
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR expands polar chart documentation and gallery navigation, adds generated API coverage for polar axes, updates interaction and rendering contracts, and extends documentation and public API validation for polar, radar, radial-bar, pie/donut, and wind-rose chart families. ChangesPolar documentation and public surface
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Merging this PR will not alter performance
Comparing Footnotes
|
b301195 to
830b333
Compare
830b333 to
c34b339
Compare
68260ed to
8711bbc
Compare
f19544d to
13c1325
Compare
c88219b to
1d057dd
Compare
e7429d8 to
e96da38
Compare
e96da38 to
407d940
Compare
407d940 to
434d323
Compare
There was a problem hiding this comment.
1 issue found across 35 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/components/tooltips.md">
<violation number="1" location="docs/components/tooltips.md:26">
P2: The new docs claim that `labels={"x": ...}` opts the suppressed polar angle back into the tooltip. The `labels` parameter only renames display labels for fields already in the default set — it does not control which fields appear. A reader trying to show the theta value would need `fields=["x"]` (or its polar equivalent) instead. Consider clarifying that `fields=` controls visibility while `labels=` only renames the display text.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Fix all with cubic | Re-trigger cubic
118edaf to
f8cf5d5
Compare
2570820 to
5973169
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
tests/test_type_surface.py (1)
363-374: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFold the required-argument cases into one mapping.
The factory names are listed twice (branch chain plus the exemption set at line 373), so adding another argument-taking factory means editing two places.
♻️ Proposed refactor
+REQUIRED_CHART_ARGS = { + "radar_chart": (["a", "b", "c"],), + "wind_rose": ([0.0], [1.0]), + "pie_chart": (["a", "b"], [1.0, 2.0]), +} + def test_chart_factories_construct_named_lazy_charts() -> None: for name in CHART_FACTORIES: - if name == "radar_chart": - chart = components.radar_chart(["a", "b", "c"]) - elif name == "wind_rose": - chart = components.wind_rose([0.0], [1.0]) - elif name == "pie_chart": - chart = components.pie_chart(["a", "b"], [1.0, 2.0]) - else: - chart = getattr(components, name)() + args = REQUIRED_CHART_ARGS.get(name, ()) + chart = getattr(components, name)(*args) assert isinstance(chart, components.Chart), name assert chart.kind == name - if name not in {"radar_chart", "wind_rose", "pie_chart"}: + if name not in REQUIRED_CHART_ARGS: assert chart.children == ()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_type_surface.py` around lines 363 - 374, Refactor the factory setup in the type-surface test to use one mapping from argument-taking names to their constructor arguments, replacing the special-case branch chain and the separate exemption set. Use the mapping both to invoke the appropriate factory and to determine whether to assert empty children, while preserving the existing calls for radar_chart, wind_rose, and pie_chart.docs/app/tests/test_docs_site.py (2)
1254-1265: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider deriving the preview count instead of repeating
34.The literal now appears at lines 1256, 1263-1265, 1276, 1347, 1512, 1783/1790/1793 and again in
docs/app/scripts/check_html_routes.py(INLINE_SVG_PREVIEW_COUNT). Computing it once from_GALLERY_GROUPSin this test file would keep future gallery additions to a single edit here.♻️ Sketch
+ preview_count = sum(len(group.items) for group in _GALLERY_GROUPS) assert len(chart_section) == 20 assert "XYChart" not in rendered - assert rendered.count("dangerouslySetInnerHTML") == 34 + assert rendered.count("dangerouslySetInnerHTML") == preview_count🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/app/tests/test_docs_site.py` around lines 1254 - 1265, Derive the expected preview count once from _GALLERY_GROUPS in the test module, then replace the repeated literal 34 assertions and related expectations with that shared value, including any matching route-check configuration that should use the same count. Preserve all existing count checks and update them automatically when gallery groups gain previews.
1563-1700: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffVery brittle prose/count assertions.
This test pins ~60 exact English fragments plus exact occurrence counts (
count("~~~python demo exec") == 4,count("xy.modebar(show=False),") == 5,'("Direct", 0, 6, "#5b3cc4")'). Any harmless copy edit or added example breaks it without signalling a real contract regression. Consider narrowing to the behavioral claims that must not drift (e.g. the deferred/stale-limitation absences and demo-ordering checks) and dropping the incidental style/color/count pins.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/app/tests/test_docs_site.py` around lines 1563 - 1700, Narrow test_polar_guides_track_the_current_coordinate_system_contract to stable behavioral assertions, retaining only meaningful stale-limitation absence checks and essential demo-ordering or presence checks. Remove brittle exact prose, styling/color literals, and occurrence-count assertions such as demo-exec, modebar, and data-value counts, while preserving checks that document required supported behavior and ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/app/tests/test_docs_site.py`:
- Around line 1459-1463: Update the page-content map in the relevant docs-site
test to assert that the /charts/pie-chart/ route contains xy.pie_chart(. Keep
the existing API-table assertion and neighboring chart mappings unchanged so the
page body and API table use the same pie-chart API.
In `@spec/api/chart-roadmap.md`:
- Around line 417-420: Update README.md lines 299-301 to document the shipped
xy.pie_chart(labels, values) API and hole= donut option instead of
xy.pie_chart(xy.pie(...)); update spec/api/chart-roadmap.md lines 453-454 if it
contains the same outdated example, while lines 417-420 require no direct change
because they already describe the correct API.
In `@spec/design/polar-axes.md`:
- Line 179: Update the fenced code block in the polar-axes documentation to
include a language identifier, preferably text, while preserving all existing
pseudo-code content unchanged.
---
Nitpick comments:
In `@docs/app/tests/test_docs_site.py`:
- Around line 1254-1265: Derive the expected preview count once from
_GALLERY_GROUPS in the test module, then replace the repeated literal 34
assertions and related expectations with that shared value, including any
matching route-check configuration that should use the same count. Preserve all
existing count checks and update them automatically when gallery groups gain
previews.
- Around line 1563-1700: Narrow
test_polar_guides_track_the_current_coordinate_system_contract to stable
behavioral assertions, retaining only meaningful stale-limitation absence checks
and essential demo-ordering or presence checks. Remove brittle exact prose,
styling/color literals, and occurrence-count assertions such as demo-exec,
modebar, and data-value counts, while preserving checks that document required
supported behavior and ordering.
In `@tests/test_type_surface.py`:
- Around line 363-374: Refactor the factory setup in the type-surface test to
use one mapping from argument-taking names to their constructor arguments,
replacing the special-case branch chain and the separate exemption set. Use the
mapping both to invoke the appropriate factory and to determine whether to
assert empty children, while preserving the existing calls for radar_chart,
wind_rose, and pie_chart.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 33b9d1c8-2a67-43e7-b48b-1799b6cfd894
📒 Files selected for processing (35)
CHANGELOG.mdREADME.mddocs/api-reference/changelog.mddocs/api-reference/chart-factories.mddocs/api-reference/limitations-and-alpha-status.mddocs/app/scripts/check_html_routes.pydocs/app/tests/test_docs_site.pydocs/app/xy_docs/api_reference.pydocs/app/xy_docs/config.pydocs/app/xy_docs/gallery.pydocs/app/xy_docs/sidebar.pydocs/charts/contour-plot.mddocs/charts/heatmap.mddocs/charts/pie-chart.mddocs/charts/polar-chart.mddocs/charts/radar-chart.mddocs/charts/radial-bar-chart.mddocs/charts/uncertainty.mddocs/charts/wind-rose.mddocs/components/annotations.mddocs/components/modebars-and-interaction-controls.mddocs/components/tooltips.mddocs/core-concepts/interactions.mddocs/core-concepts/large-data-and-performance.mddocs/guides/display-and-export.mddocs/integrations/matplotlib.mddocs/overview/gallery.mdpython/xy/components.pyscripts/check_public_api.pyspec/api/chart-kind-contract.mdspec/api/chart-roadmap.mdspec/api/interaction.mdspec/design/polar-axes.mdspec/matplotlib/compat.mdtests/test_type_surface.py
5973169 to
16f642b
Compare
Part 2 of the stack:
Summary
base=0pie driven by an externalPIE_DATAvariable; angles, widths, percentages, and native legend labels update from the raw values#6e56cfxy.modebar(show=False)while retaining hover, wheel zoom, reset, and export APIsradar_chart(fill=False), including line color, width, opacity, curve, and dash inheritanceRadial Bar block details
RADIAL_DATA, six named purple-shaded bars, degree and radial axes, a right-side legend, and no toolbarPie block details
The polished blocks keep exportable chart geometry inside XY and compose dashboard UI such as center labels and legends with Reflex. Hidden theta/r axes use
tick_label_strategy="none"so the chart canvas fills each card without invisible tick gutters.Stack
alek/polar-axes(PR Polar coordinate system: heatmap, sectors, log r, and error bars (protocol v12) #370)agent/polar-chart-docs(this PR)ee413d6de96da38e), preserving the Polar coordinate system: heatmap, sectors, log r, and error bars (protocol v12) #370 → Docs: add polar, radar, radial bar, pie, and wind rose guides #371 stackValidation
Summary by CodeRabbit
radar_chart(fill=False)so outlines render correctly using compatible line styling.